Skip to content

fix: allow partial classes prop in withStyles#1428

Merged
kof merged 1 commit intocssinjs:masterfrom
dru89:master
Feb 15, 2021
Merged

fix: allow partial classes prop in withStyles#1428
kof merged 1 commit intocssinjs:masterfrom
dru89:master

Conversation

@dru89
Copy link
Copy Markdown
Contributor

@dru89 dru89 commented Nov 21, 2020

The classes prop created by the withStyles function is meant to be an optional parameter where you can provide some or all of the style classes.

Imagine a somewhat contrived snippet like:

import React from "react";
import withStyles, { WithStylesProps } from "react-jss";
import clsx from "clsx";

const styles = {
  root: { display: "block" },
  blue: { color: "blue" },
};

type Props = { blue?: boolean } & WithStylesProps<typeof styles>;
const Component = ({ blue = false, classes }: Props) => (
  <div className={clsx(classes.root, { [classes.blue]: blue })}>
    Hello, world!
  </div>
);

const StyledComponent = withStyles(styles)(Component);

You'd expect to be able to use it in any of the following ways:

<StyledComponent />
<StyledComponent blue />
<StyledComponent classes={{ root: "myClass" }} />

The last use case was broken in TypeScript because, while classes was an optional prop, each key within class was required. So you'd get an error like:

Property 'blue' is missing in type '{ root: string; }' but required in type 'Record<"blue" | "root", string>'.ts(2741)

This changes makes sure that the classes property is still optional, but that each key within it is optional, too. I abstracted a field out so that I could use it in two places. There's probably about a dozen ways to set something like this up, though, so if you'd prefer it formatted in a different way let me know.

The `classes` prop created by the `withStyles` function is meant to be
an optional parameter where you can provide some or all of the
style classes.
@dru89 dru89 requested a review from HenriBeck as a code owner November 21, 2020 06:32
@dru89
Copy link
Copy Markdown
Contributor Author

dru89 commented Nov 21, 2020

As a side note, I tried following all of the steps in CONTRIBUTING.md, but several of the steps don't seem to work anymore.

There's no item in package.json for yarn lint or yarn typecheck. If there's something else I should do to check these files, let me know. I did run yarn build and yarn format, though since the TypeScript files are separate from the main code I'm not sure what else I should be looking for. 😄

@kof kof added the typescript label Nov 21, 2020
@kof kof requested review from moshest and removed request for HenriBeck November 21, 2020 11:16
@kof
Copy link
Copy Markdown
Member

kof commented Nov 21, 2020

makes sense to me

@kof kof requested review from moshest and removed request for moshest February 15, 2021 11:17
Copy link
Copy Markdown
Member

@moshest moshest left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@kof kof merged commit a27b9ac into cssinjs:master Feb 15, 2021
@kof
Copy link
Copy Markdown
Member

kof commented Feb 15, 2021

merged, thanks

@kof
Copy link
Copy Markdown
Member

kof commented Mar 14, 2021

Released in v10.6.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants